fix: forward isInvalid to Select and DatePicker trigger ring - #199
Draft
simonyang08 wants to merge 1 commit into
Draft
fix: forward isInvalid to Select and DatePicker trigger ring#199simonyang08 wants to merge 1 commit into
simonyang08 wants to merge 1 commit into
Conversation
…ntitleduico#187) The Select trigger button hardcoded its ring color and only branched styling on isFocused/isOpen/isDisabled. The DatePicker trigger used a generic Button that had no concept of invalid state at all. In both cases the isInvalid prop was forwarded to the underlying react-aria root for assistive tech but never reached the visible trigger, so users couldn't tell at a glance which field was in error. Add the error ring token (matching the existing Input convention) to both triggers. The DatePicker now reads isInvalid from DatePickerStateContext to keep the data flow local to react-aria. Adds vitest + @testing-library/react + jsdom as devDeps and a single regression test that asserts the trigger button's className contains ring-error_subtle when isInvalid is set, and does not when it is not. Signed-off-by: simonyang08 <ppt5928@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #187
Description
SelectandDatePickeracceptisInvalid, whichreact-aria-componentsforwards to the root (soaria-invalidanddata-invalidare set for assistive tech). However, the visible trigger button rendered by each component never received the invalid state in itsclassName, so the ring stayed neutral even when validation had failed. Sighted users had no way to tell at a glance which field was in error.This PR aligns both triggers with the convention already used by
Input(seecomponents/base/input/input.tsx:109-114):Select: threadstate.isInvalidintoSelectValue, addring-error_subtleat rest andring-2 ring-erroron focus/open.DatePicker: readisInvalidfromDatePickerStateContextand applyring-error_subtleto the triggerButton.Changes
components/base/select/select.tsx— destructureisInvalidfrom the spread{...state}and add the error ring classes to the trigger button.components/application/date-picker/date-picker.tsx— introduce a smallDatePickerTriggercomponent that consumesDatePickerStateContextand forwardsisInvalidto the triggerButton.tests/invalid-ring.test.tsx(new) — vitest +@testing-library/reactregression test asserting thatring-error_subtleappears on the trigger button whenisInvalidis set and does not appear when it is not.vitest.config.mts(new) + 3 devDeps inpackage.json— minimal test infrastructure. The repo did not previously ship a unit test runner.Testing
vitest run tests/invalid-ring.test.tsx)tscerrors (3 pre-existing errors are unrelated to this change).eslinterrors (0 errors repo-wide; only CRLF warnings, consistent with the existing source-tree line-ending style).prettierwarnings.Related issues
Scope note
The same invalid-ring gap exists in
MultiSelect,TagSelect, andComboBox(verified via grep onring-1 ring-primarybranches). Those are intentionally out of scope here to keep the diff under the repo's 5-file / 200-LOC convention. Happy to follow up with a separate PR once this lands.